fix(http-bridge): re-check liveness before retiring a stale pending session - #1649

@codex review

Thanks for the fast iterations on the codex P1s — the lock-ordering fix (sampling pending state before _http_bridge_lock) and the upstream_close_attempted guard on the retirement path are the right shape. But the liveness signal added in the last two commits overshoots, and I believe it introduces a worse failure mode than the race being fixed:

1. last_completed_response_id is eternal history, not "became healthy during suspension" (app/modules/proxy/_service/http_bridge/request_submit.py:1836-1841)

became_healthy_during_suspend treats session.last_completed_response_id is not None (with a caller snapshot of 0) as proof that a response arrived during the retry-circuit await. But that field is set on every response.completed (upstream_events.py:2018) and is only cleared on account swap (mixin.py:2369) — it persists across turns for the life of the session. So for any session that has ever completed a turn (the normal case for reused bridge sessions):

• _fail_http_bridge_reader_and_maybe_retire sets session.closed = True because the upstream reader died, fails the pending requests, then calls retirement with response_events_seen=0 (an eventless new turn, e.g. stream_incomplete). The recheck sees the old completed id, flips session.closed = False, clears reconnect_requested/retire_after_drain, and returns without ever reaching _close_http_bridge_session_bounded. The session stays registered as open and reusable with a dead upstream websocket, and its account lease / durable ownership are never released. Each request routed to it fails, re-enters the same path, and gets reopened again — a dead-session leak that persists until account swap or restart.
• The stuck-session path at app/modules/proxy/service.py:1381 (response_create_gate_timeout_stuck_pending) passes no response_events_seen, so it is similarly neutered for any multi-turn session.

The codex finding you were addressing asked for evidence that is new since the caller's snapshot. Snapshotting session.last_completed_response_id at the top of _retire_stale_pending_http_bridge_session (before the retry-circuit await) and comparing it to the value at decision time would give exactly that: completed_at_entry != completed_now ⇒ a terminal response landed during suspension. The current_response_events_seen > caller_response_events_seen term is fine; the bare is not None term is the problem.

2. Split-lock claim of upstream_close_attempted

The close claim moved under _http_bridge_lock (request_submit.py:1866-1868), but _retire_http_bridge_after_drain_if_ready still claims the same flag under session.pending_lock (request_submit.py:1800-1806). Two racing paths can now both observe False and both invoke _close_http_bridge_session_bounded. If bounded close is fully idempotent this may be benign, but the claim used to be serialized by a single lock — either keep the claim under pending_lock or document why a double close is safe.

3. Stale-base artifact: app/modules/api_keys/repository.py

update_last_used was intentionally removed on main by the merged coalesce-api-key-last-used-writes change (its tasks.md 2.2: "remove the now-unused ApiKeysRepository.update_last_used and its protocol entry"). This branch predates that removal and re-adds the method; nothing on current main references it. Please rebase onto current main and drop this hunk. (The db migration label is also stale from an earlier head — the current diff contains no alembic files.)

4. Test gap

Both new tests model callers with response_events_seen=0 and last_completed_response_id=None. A control like "session completed turn 1, upstream reader dies with an eventless turn 2 pending → session must still retire and release its lease" would have caught #1 and would make a good regression for the rework.

One more process note: the last codex review ran on 1966752; the final two commits have not had a codex pass. After the rework, please re-request @codex review so the label gate can go green.

@codex review

Thanks — verified at head 378bd09 that all three prior blockers are resolved in code: the liveness signal now compares last_completed_response_id against a baseline snapshotted under pending_lock at entry (no more eternal-history false positive), the upstream_close_attempted claim is serialized under _http_bridge_lock, and the new last_upstream_event_generation counter closes the sampling/close race from the 08-14 review. Three things remain before this can be re-reviewed for merge:

1. Merge conflicts — main has since landed fix(proxy): fence successor bridge claims against the retiring predecessor #1751 (successor-claim fencing), fix(proxy): sweep idle bridge sessions without request traffic #1747 (idle bridge sweep) and fix(http-bridge): preserve goal-restart recovery across reconnects #1680, all touching request_submit.py/support.py/test_proxy_http_bridge.py. Please rebase; note the retirement path itself was refactored by fix(proxy): fence successor bridge claims against the retiring predecessor #1751/fix(proxy): sweep idle bridge sessions without request traffic #1747, so the recheck placement needs care during the rebase, not just mechanical conflict resolution.
2. Unit CI is red — tests/unit/test_otel.py::test_lifespan_drains_actual_audit_and_cancelled_fleet_tasks_before_resource_close timed out on this head. It looks unrelated to your change, but we need a green run on the rebased head to know.
3. Out-of-scope hunk — commit c13f381 (app/modules/api_keys/repository.py last-used touch) is the same stray hunk flagged on fix(ops,config,quota): compose stop grace, refresh timeout bounds, expirable warmup claim #1646 and has no connection to bridge retirement. Please drop it from this branch.

Happy to do a full re-review as soon as the rebased head is green.

Rebase nudge: this was substantively approved on 08-16 pending a careful rebase — the offer of a prompt full re-review as soon as a rebased head is green still stands. Highest-leverage branch in your open batch, worth doing first.

_{Comment @coderabbitai help to get the list of available commands.}